-
Notifications
You must be signed in to change notification settings - Fork 0
Feat/#17 home api 연동 #20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
The head ref may contain hidden characters: "feat/#17-home_api_\uC5F0\uB3D9"
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @rhkrwngud445, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
이 Pull Request는 홈 화면을 백엔드 API와 통합하여 친구 및 월별 연락처 데이터를 표시하는 데 중점을 둡니다. 이는 더 견고한 데이터 계층을 위해 새로운 엔티티와 매퍼를 도입하는 등 데이터 모델의 상당한 리팩토링을 포함합니다. 또한, 친구 추가와 같은 작업을 위한 드롭다운 메뉴를 추가하고 친구 프로필로의 내비게이션을 개선하여 사용자 인터페이스를 향상시킵니다.
Highlights
- 데이터 모델 재구성: 기존
Friend및ContactSummary모델을 새로운FriendSummary데이터 클래스로 대체하고, 월별 연락처를 위한MonthlyFriend및MonthlyFriendType을 도입하여 데이터 모델을 재구성했습니다. - API 통합: 월별 친구 데이터를 가져오는 새로운 API 호출 및 리포지토리 메서드를 추가했으며, 기존 친구 데이터 가져오기 기능을 업데이트된
FriendSummary모델을 사용하도록 변경했습니다. - UI 개선: 홈 화면에 '친구 추가'와 같은 작업을 위한 드롭다운 메뉴를 구현하고, 월별 연락처의 아이콘을 유형에 따라 동적으로 표시하도록 업데이트하여 사용자 인터페이스를 개선했습니다.
- 내비게이션 개선:
navigateToFriendProfile함수를 수정하여friendId를 매개변수로 받도록 함으로써, 홈 화면에서 특정 친구의 프로필로 직접 이동할 수 있도록 내비게이션 기능을 향상시켰습니다.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
이번 PR은 홈 화면 API 연동 및 관련 UI/로직 수정을 다루고 있습니다. 전반적으로 기능 구현은 잘 이루어졌지만, 몇 가지 개선점을 제안합니다. 특히, 내비게이션 시 friendId가 전달되지 않는 치명적인 버그가 있으며, ViewModel에서 예외를 적절히 처리하지 않아 잠재적인 문제를 야기할 수 있는 부분이 있습니다. 또한 코드 일관성 및 가독성을 높일 수 있는 몇 가지 스타일 관련 제안도 포함했습니다. 자세한 내용은 각 파일의 주석을 확인해주세요.
| object RouteFriendProfile | ||
|
|
||
| fun NavController.navigateToFriendProfile(navOptions: NavOptions) { | ||
| fun NavController.navigateToFriendProfile( | ||
| friendId: String, | ||
| navOptions: NavOptions? = null, | ||
| ) { | ||
| navigate(RouteFriendProfile, navOptions) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
navigateToFriendProfile 함수가 friendId를 인자로 받지만, 실제 navigate 호출 시에는 사용하지 않고 있습니다. 이로 인해 친구 프로필 화면으로 이동할 때 어떤 친구의 프로필을 보여줘야 할지 알 수 없습니다.
이를 해결하려면 RouteFriendProfile을 object에서 friendId를 가질 수 있는 data class로 변경하고, navigate 함수 호출 시 friendId를 전달해야 합니다.
| object RouteFriendProfile | |
| fun NavController.navigateToFriendProfile(navOptions: NavOptions) { | |
| fun NavController.navigateToFriendProfile( | |
| friendId: String, | |
| navOptions: NavOptions? = null, | |
| ) { | |
| navigate(RouteFriendProfile, navOptions) | |
| } | |
| @Serializable | |
| data class RouteFriendProfile(val friendId: String) | |
| fun NavController.navigateToFriendProfile( | |
| friendId: String, | |
| navOptions: NavOptions? = null, | |
| ) { | |
| navigate(RouteFriendProfile(friendId), navOptions) | |
| } |
| val friendsFlow: StateFlow<List<FriendSummary>> = | ||
| friendRepository | ||
| .fetchFriends() | ||
| .catch { } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| StateFlow<List<MonthlyFriend>> = | ||
| friendRepository | ||
| .fetchMonthlyFriends() | ||
| .catch { } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| in 0..29 -> ContactFrequency.LOW | ||
| in 30..69 -> ContactFrequency.MIDDLE | ||
| in 70..100 -> ContactFrequency.HIGH | ||
| else -> ContactFrequency.LOW |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
checkRate가 0..100 범위를 벗어나는 경우를 else로 처리하여 ContactFrequency.LOW를 반환하고 있습니다. 만약 checkRate가 항상 0에서 100 사이의 값이라고 보장된다면 이 else 구문은 필요 없습니다. 만약 범위를 벗어나는 값이 들어올 수 있다면, 이는 예외적인 상황일 수 있으므로 IllegalArgumentException을 던져서 잘못된 데이터가 시스템에 유입되는 것을 방지하는 것이 더 안전한 방법일 수 있습니다.
| else -> ContactFrequency.LOW | |
| else -> throw IllegalArgumentException("checkRate must be between 0 and 100, but was $checkRate") |
| } | ||
|
|
||
| companion object { | ||
| companion object Companion { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| fun from(value: String): MonthlyFriendType = | ||
| entries.firstOrNull { it.name == value } | ||
| ?: throw IllegalArgumentException( | ||
| ERROR_MESSAGE_NOT_FOUND_MONTHLY_TYPE, | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
문자열로부터 enum 값을 찾는 현재 구현은 entries.firstOrNull를 사용하고 있습니다. Kotlin에서는 valueOf() 함수를 사용하여 더 간결하고 관용적으로 구현할 수 있습니다. valueOf()는 대소문자를 구분하므로, API 응답의 대소문자 정책에 따라 uppercase()를 사용하여 유연성을 높일 수 있습니다.
| fun from(value: String): MonthlyFriendType = | |
| entries.firstOrNull { it.name == value } | |
| ?: throw IllegalArgumentException( | |
| ERROR_MESSAGE_NOT_FOUND_MONTHLY_TYPE, | |
| ) | |
| fun from(value: String): MonthlyFriendType = try { | |
| valueOf(value.uppercase()) | |
| } catch (e: IllegalArgumentException) { | |
| throw IllegalArgumentException(ERROR_MESSAGE_NOT_FOUND_MONTHLY_TYPE, e) | |
| } |
| @GET("/friend/list") | ||
| suspend fun fetchFriends(): List<FriendEntity> | ||
|
|
||
| @GET("friend/monthly") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Image( | ||
| modifier = | ||
| Modifier.onNoRippleClick(onClick = { | ||
| Log.d("covy", "onClick") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Image( | ||
| modifier = | ||
| Modifier.onNoRippleClick(onClick = { | ||
| Log.d("covy", "onClick") | ||
| dropdownState.value = true | ||
| }), | ||
| painter = painterResource(R.drawable.ic_32_menu), | ||
| contentDescription = stringResource(R.string.home_my_people_setting), | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
클릭 가능한 아이콘을 구현하기 위해 Image와 커스텀 onNoRippleClick 수정자를 사용하고 있습니다. IconButton과 Icon 컴포저블을 사용하는 것이 더 관용적이며 접근성 및 터치 영역 처리에 더 좋습니다. IconButton은 기본적으로 리플 효과를 제공하지만, 필요하다면 커스터마이징하여 제거할 수 있습니다.
| Image( | |
| modifier = | |
| Modifier.onNoRippleClick(onClick = { | |
| Log.d("covy", "onClick") | |
| dropdownState.value = true | |
| }), | |
| painter = painterResource(R.drawable.ic_32_menu), | |
| contentDescription = stringResource(R.string.home_my_people_setting), | |
| ) | |
| IconButton(onClick = { | |
| Log.d("covy", "onClick") | |
| dropdownState.value = true | |
| }) { | |
| Icon( | |
| painter = painterResource(R.drawable.ic_32_menu), | |
| contentDescription = stringResource(R.string.home_my_people_setting), | |
| ) | |
| } |
작업 내용
확인 방법
Preview 및 Run
참고 사항
관련 이슈